Skip to content

inspector: auto collect webstorage data#62145

Open
islandryu wants to merge 6 commits intonodejs:mainfrom
islandryu:autoCollectWebStorage
Open

inspector: auto collect webstorage data#62145
islandryu wants to merge 6 commits intonodejs:mainfrom
islandryu:autoCollectWebStorage

Conversation

@islandryu
Copy link
Member

@islandryu islandryu commented Mar 7, 2026

Web Storage data collection, which previously required explicitly firing events to send data to DevTools, is now performed automatically within Node.js.
Web Storage will appear in DevTools simply by specifying --experimental-storage-inspection, without requiring any code changes.

This feature relies on the changes introduced in the following change request, so it can be verified with the latest DevTools commit:
https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7274801

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/inspector

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Mar 7, 2026
@codecov
Copy link

codecov bot commented Mar 7, 2026

Codecov Report

❌ Patch coverage is 93.37017% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.65%. Comparing base (a6e9e32) to head (5debe1c).
⚠️ Report is 58 commits behind head on main.

Files with missing lines Patch % Lines
src/inspector/dom_storage_agent.cc 82.92% 1 Missing and 6 partials ⚠️
src/node_webstorage.cc 77.27% 1 Missing and 4 partials ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #62145    +/-   ##
========================================
  Coverage   89.64%   89.65%            
========================================
  Files         676      677     +1     
  Lines      206240   206709   +469     
  Branches    39514    39579    +65     
========================================
+ Hits       184891   185329   +438     
+ Misses      13465    13457     -8     
- Partials     7884     7923    +39     
Files with missing lines Coverage Δ
lib/internal/inspector/webstorage.js 100.00% <100.00%> (ø)
lib/internal/webstorage.js 100.00% <100.00%> (ø)
src/node_builtins.cc 76.14% <ø> (+0.19%) ⬆️
src/node_webstorage.h 83.33% <ø> (ø)
src/node_webstorage.cc 74.87% <77.27%> (+0.13%) ⬆️
src/inspector/dom_storage_agent.cc 68.08% <82.92%> (+18.40%) ⬆️

... and 70 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

I'd still recommend adding a test for UTF-16 characters outside the ISO-8859-1 range (e.g. emoji or Chinese characters)

@addaleax addaleax added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 8, 2026
@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Mar 8, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Mar 8, 2026

Failed to start CI
- Validating Jenkins credentials
✔  Jenkins credentials valid
- Starting PR CI job
✘  Failed to start PR CI: 404 Not Found
https://github.com/nodejs/node/actions/runs/22820941170

@aduh95
Copy link
Contributor

aduh95 commented Mar 8, 2026

Related test failure:

Path: parallel/test-code-cache
Error: --- stderr ---
node:internal/bootstrap/realm:185
      mod = bindingObj[module] = getInternalBinding(module);
                                 ^

Error: No such binding: webstorage

You probably need to skip that test when compiled without SQLite

@islandryu islandryu force-pushed the autoCollectWebStorage branch from 8e0c41b to b278ecc Compare March 9, 2026 04:34
@islandryu
Copy link
Member Author

I changed the implementation to handle the storage keys and values as UTF-16.

@islandryu islandryu force-pushed the autoCollectWebStorage branch from b278ecc to 8fc09a7 Compare March 9, 2026 05:21
@nodejs-github-bot
Copy link
Collaborator

Comment on lines +256 to +259
storage_map[std::u16string(reinterpret_cast<const char16_t*>(*key_utf16),
key_utf16.length())] =
std::u16string(reinterpret_cast<const char16_t*>(*value_utf16),
value_utf16.length());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
storage_map[std::u16string(reinterpret_cast<const char16_t*>(*key_utf16),
key_utf16.length())] =
std::u16string(reinterpret_cast<const char16_t*>(*value_utf16),
value_utf16.length());
storage_map[key_utf16.ToU16String()] = value_utf16.ToU16String();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants